Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat : upgrade to spring boot 3.4.0 and delombok #1556

Merged
merged 2 commits into from
Dec 5, 2024
Merged

Conversation

rajadilipkolli
Copy link
Owner

@rajadilipkolli rajadilipkolli commented Dec 5, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a new unit test for the RestaurantRequest class to validate JSON serialization and deserialization.
  • Improvements

    • Updated dependencies to newer versions for Spring Boot and Springdoc OpenAPI, potentially enhancing performance and security.
    • Modified configuration classes to improve dependency injection and proxy behavior.
    • Enhanced explicitness of property accessors in the ApplicationProperties and Address classes.
  • Bug Fixes

    • Adjusted query handling in the CustomRestaurantESRepositoryImpl for better range query processing.
  • Refactor

    • Removed unnecessary annotations and explicitly defined constructors in several configuration classes to improve clarity and maintainability.
    • Transitioned from Lombok annotations to manual implementations for constructors and accessors in various classes for better control over behavior.

@rajadilipkolli rajadilipkolli self-assigned this Dec 5, 2024
Copy link
Contributor

coderabbitai bot commented Dec 5, 2024

Walkthrough

The pull request includes updates to various Java classes and the pom.xml file in the boot-mongodb-elasticsearch project. Key changes involve upgrading Spring Boot and Springdoc OpenAPI versions, modifying access modifiers for several configuration classes to package-private, and replacing Lombok annotations with explicit constructors. Additionally, the Address class has had its JSON deserialization annotation removed, and a new unit test for the RestaurantRequest class has been introduced.

Changes

File Change Summary
boot-mongodb-elasticsearch/pom.xml Updated Spring Boot version from 3.3.6 to 3.4.0, Springdoc OpenAPI from 2.6.0 to 2.7.0, and Google Java Format plugin from 1.23.0 to 1.25.0. Removed jaxb-runtime and lombok dependencies.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/DataStoreConfiguration.java Removed public modifier, making DataStoreConfiguration package-private.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/Initializer.java Removed @RequiredArgsConstructor and added explicit constructor with RestaurantService parameter.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/SwaggerConfig.java Updated @Configuration to include proxyBeanMethods = false and changed access modifier to package-private.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/WebFluxConfig.java Removed @RequiredArgsConstructor, added explicit constructor, updated @Configuration, and added @Import(SpringDataJacksonConfiguration.class).
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Address.java Removed @JsonDeserialize(as = Point.class) annotation from location field and added explicit getters and setters for all fields.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/elasticsearch/repository/CustomRestaurantESRepositoryImpl.java Removed @RequiredArgsConstructor, added explicit constructor, and modified query handling in searchRestaurantIdRange and searchDateRange methods.
boot-mongodb-elasticsearch/src/test/java/com/example/mongoes/web/model/RestaurantRequestTest.java Added new unit test class for RestaurantRequest focusing on JSON serialization and deserialization.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/ApplicationProperties.java Removed @Data annotation, added explicit getters and setters for cors property and its inner Cors class properties.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/ChangeStreamResume.java Removed @Getter and @Setter annotations, added explicit getters and setters for id and resumeTimestamp fields.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Grades.java Removed Lombok annotations, added explicit constructors, getters, setters, and a custom toString() method.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Restaurant.java Removed Lombok annotations, added explicit constructors, getters, setters, and a custom toString() method.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/mongodb/repository/CustomChangeStreamResumeRepositoryImpl.java Removed @RequiredArgsConstructor, added explicit constructor for dependency injection.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/utils/DateUtility.java Removed @UtilityClass annotation, updated method to public static.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/controller/RestaurantController.java Removed @RequiredArgsConstructor, added explicit constructor for dependency injection.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/controller/SearchController.java Removed @RequiredArgsConstructor, added explicit constructor for dependency injection.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/service/RestaurantService.java Removed @Slf4j and @RequiredArgsConstructor, added explicit constructor and static logger.
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/service/SearchService.java Removed @RequiredArgsConstructor, added explicit constructor, and updated string utility for sorting logic.

Possibly related PRs

Poem

🐰 In the code where rabbits play,
Upgraded versions lead the way.
With constructors clear and bright,
Configuration's now just right.
JSON dances, tests take flight,
Hooray for changes, what a sight! 🎉


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added type: dependency-upgrade Pull requests that update a dependency file component: mongodb-elasticsearch labels Dec 5, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (4)
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/elasticsearch/repository/CustomRestaurantESRepositoryImpl.java (1)

272-354: Documentation improvements needed

The added query example is helpful, but there are a few issues to address:

  1. Typo in field name: "restautant_name" should be "restaurant_name"
  2. Date format mismatch: Example shows "dd-MM-yyy'T'hh:mm:ss" but code uses "dd-MM-yyy"
-     * "restautant_name",
+     * "restaurant_name",
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/SwaggerConfig.java (1)

10-12: Good improvements to configuration class!

The changes align with Spring Boot best practices:

  1. Setting proxyBeanMethods = false improves startup time and memory usage since this configuration doesn't require bean method interception
  2. Reducing visibility to package-private is appropriate as this configuration doesn't need to be accessed outside its package

Consider applying these same optimizations to other @Configuration classes in the project for consistency and performance benefits.

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/DataStoreConfiguration.java (1)

28-28: LGTM! Good practice to make configuration classes package-private.

Making configuration classes package-private helps enforce proper modularity by preventing unintended access from other packages. This change aligns with the principle of encapsulation.

Consider applying this pattern consistently across all configuration classes in the application for better maintainability.

boot-mongodb-elasticsearch/src/test/java/com/example/mongoes/web/model/RestaurantRequestTest.java (1)

25-45: Enhance test coverage with additional scenarios.

While the basic serialization test is good, consider adding:

  1. Edge cases (null values, empty lists)
  2. Field validation tests
  3. JSON structure verification
  4. Custom field format validation (e.g., date format)

Example enhancement:

 @Test
 void serializeInCorrectFormat() throws IOException {
     // ... existing test code ...
     
     JsonContent<RestaurantRequest> json = jacksonTester.write(restaurantRequest);
     assertThat(json.getJson()).isNotNull();
+    // Verify JSON structure
+    assertThat(json).hasJsonPathNumberValue("$.id");
+    assertThat(json).hasJsonPathStringValue("$.name");
+    assertThat(json).hasJsonPathArrayValue("$.grades");
+    
+    // Verify field values
+    assertThat(json).extractingJsonPathNumberValue("$.id").isEqualTo(1);
+    assertThat(json).extractingJsonPathStringValue("$.name").isEqualTo("junitRestaurant");
 
     RestaurantRequest orderRequest = jacksonTester.parseObject(json.getJson());
     assertThat(orderRequest).isNotNull();
+    assertThat(orderRequest.getId()).isEqualTo(1L);
+    assertThat(orderRequest.getName()).isEqualTo("junitRestaurant");
 }
+
+@Test
+void shouldHandleNullValues() throws IOException {
+    RestaurantRequest request = new RestaurantRequest(1L, null, null, null, null, null);
+    JsonContent<RestaurantRequest> json = jacksonTester.write(request);
+    RestaurantRequest parsed = jacksonTester.parseObject(json.getJson());
+    assertThat(parsed.getName()).isNull();
+    assertThat(parsed.getGrades()).isNull();
+}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 60f0a62 and 06741bf.

📒 Files selected for processing (8)
  • boot-mongodb-elasticsearch/pom.xml (3 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/DataStoreConfiguration.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/Initializer.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/SwaggerConfig.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/WebFluxConfig.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Address.java (0 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/elasticsearch/repository/CustomRestaurantESRepositoryImpl.java (3 hunks)
  • boot-mongodb-elasticsearch/src/test/java/com/example/mongoes/web/model/RestaurantRequestTest.java (1 hunks)
💤 Files with no reviewable changes (1)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Address.java
🔇 Additional comments (6)
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/elasticsearch/repository/CustomRestaurantESRepositoryImpl.java (2)

35-38: LGTM: Constructor properly replaces Lombok annotation

The explicit constructor correctly initializes the required dependency, maintaining the same functionality as the previous @RequiredArgsConstructor annotation.


237-245: LGTM: Range query implementations improved

The changes improve type safety:

  • Using String.valueOf() for numeric range values ensures proper type conversion
  • Date range query structure is more concise and aligned with current Elasticsearch client API

Let's verify the range query functionality:

Also applies to: 260-264

✅ Verification successful

Range query implementations are properly tested and verified

The integration tests confirm the functionality:

  • searchRestaurantIdRange test verifies numeric range queries with proper assertions on result size and pagination
  • searchDateRange test validates datetime range queries with comprehensive result validation
  • Both tests use StepVerifier for reactive flow verification and include thorough assertions on the search results
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any existing tests covering these range queries

# Search for test files that might cover these methods
rg -l "searchRestaurantIdRange|searchDateRange" --type java

Length of output: 1134


Script:

#!/bin/bash
# Check the test implementations to verify range query testing
rg "searchRestaurantIdRange|searchDateRange" -B 2 -A 10 boot-mongodb-elasticsearch/src/test/java/com/example/mongoes/elasticsearch/repository/RestaurantESRepositoryIntegrationTest.java

Length of output: 2214

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/WebFluxConfig.java (2)

9-9: Good configuration improvements!

The changes improve code clarity and performance:

  1. Setting proxyBeanMethods = false optimizes Spring bean creation
  2. Explicit constructor is more maintainable than Lombok's @RequiredArgsConstructor

Also applies to: 15-17


10-10: Verify SpringDataJacksonConfiguration import necessity

The addition of @Import(SpringDataJacksonConfiguration.class) should be verified as it might affect JSON serialization behavior.

Let's check if this is a required change for Spring Boot 3.4.0:

✅ Verification successful

Let me gather more information about the usage context:


Based on the gathered information, I can now provide the final response:

SpringDataJacksonConfiguration import is correctly added

The @Import(SpringDataJacksonConfiguration.class) is necessary as this application uses Spring Data's Point class in the domain model, which requires proper Jackson serialization support. This is evidenced by:

  • The test class RestaurantRequestTest.java also imports the same configuration for JSON serialization tests
  • The test verifies serialization of Point class from org.springframework.data.geo.Point
  • Spring Boot 3.4.0 includes Jackson BOM upgrade, making this explicit configuration important for proper geo-spatial type handling
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for similar imports in the codebase and changelog
rg -l "SpringDataJacksonConfiguration" | grep -v "WebFluxConfig.java"

# Check Spring Boot 3.4.0 changelog for related changes
gh api repos/spring-projects/spring-boot/releases/tags/v3.4.0 --jq '.body' | grep -i "jackson"

Length of output: 360


Script:

#!/bin/bash
# Check the test file that imports SpringDataJacksonConfiguration
cat boot-mongodb-elasticsearch/src/test/java/com/example/mongoes/web/model/RestaurantRequestTest.java

# Look for Jackson-related configuration or beans in the codebase
rg -l "ObjectMapper|Jackson" --type java

# Check if there are any explicit Jackson configurations
ast-grep --pattern 'class $_ implements WebFluxConfigurer { $$$ }'

Length of output: 9846

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/Initializer.java (1)

20-22: Good constructor refactoring!

The explicit constructor replacement for @RequiredArgsConstructor improves code clarity and maintainability. This change is consistent with the project's apparent move away from Lombok constructor annotations.

boot-mongodb-elasticsearch/pom.xml (1)

22-22: Verify compatibility with updated dependencies.

The PR updates multiple dependencies:

  • springdoc-openapi from 2.6.0 to 2.7.0
  • google-java-format from 1.23.0 to 1.25.0

Let's verify compatibility and check for any known issues:

Also applies to: 232-232

✅ Verification successful

Springdoc-openapi 2.7.0 upgrade is compatible with Spring Boot 3.4

The upgrade from springdoc-openapi 2.6.0 to 2.7.0 is safe as:

  • It officially supports Spring Boot 3.4.0 as per the changelog
  • Contains multiple bug fixes and improvements
  • No critical issues or compatibility problems reported with Spring Boot 3.4
  • Includes important fixes for Swagger UI and API documentation generation
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for compatibility issues in dependency upgrades

# Check springdoc-openapi release notes
echo "Checking springdoc-openapi 2.7.0 compatibility..."
gh api repos/springdoc/springdoc-openapi/releases/tags/v2.7.0 --jq '.body'

# Check for any reported issues
echo "Checking for reported issues..."
gh api graphql -f query='
{
  search(query: "repo:springdoc/springdoc-openapi is:issue label:bug springboot-3.4", type: ISSUE, first: 5) {
    nodes {
      ... on Issue {
        title
        url
        state
      }
    }
  }
}'

Length of output: 6507

@@ -5,7 +5,7 @@
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.3.6</version>
<version>3.4.0</version>
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Caution: Using Spring Boot milestone version in production.

Spring Boot 3.4.0 appears to be a milestone version (based on the Spring Milestones repository). Milestone versions may contain breaking changes and are not recommended for production use.

Consider:

  1. Waiting for the GA release
  2. Or staying on the latest stable version (3.3.x)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai, spring boot 3.4.0 is released on 21st November 2024, so its a valid version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajadilipkolli, thank you for the clarification. Since Spring Boot 3.4.0 was released on 21st November 2024 and is a valid version, your upgrade is appropriate.


✏️ Learnings added
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1556
File: boot-mongodb-elasticsearch/pom.xml:8-8
Timestamp: 2024-12-05T12:32:53.805Z
Learning: Spring Boot 3.4.0 was released on 21st November 2024 and is considered a stable release suitable for production use.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link

sonarqubecloud bot commented Dec 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
19.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@rajadilipkolli rajadilipkolli changed the title feat : upgrade to spring boot 3.4.0 feat : upgrade to spring boot 3.4.0 and delombok Dec 5, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (9)
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/mongodb/repository/CustomChangeStreamResumeRepositoryImpl.java (1)

17-19: LGTM! Consider adding parameter validation.

The explicit constructor replacement for Lombok's @requiredargsconstructor is clean and appropriate. While Spring ensures non-null dependency injection, consider adding parameter validation as a defensive programming practice.

 public CustomChangeStreamResumeRepositoryImpl(ReactiveMongoTemplate reactiveMongoTemplate) {
+    if (reactiveMongoTemplate == null) {
+        throw new IllegalArgumentException("reactiveMongoTemplate must not be null");
+    }
     this.reactiveMongoTemplate = reactiveMongoTemplate;
 }
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/ChangeStreamResume.java (1)

8-28: Consider adding equals, hashCode, and toString methods.

As this is a MongoDB document class, it would be beneficial to implement:

  1. equals/hashCode - for proper document comparison and collection operations
  2. toString - for better debugging and logging

Here's a suggested implementation:

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof ChangeStreamResume that)) return false;
    return Objects.equals(id, that.id) && 
           Objects.equals(resumeTimestamp, that.resumeTimestamp);
}

@Override
public int hashCode() {
    return Objects.hash(id, resumeTimestamp);
}

@Override
public String toString() {
    return "ChangeStreamResume{" +
           "id='" + id + '\'' +
           ", resumeTimestamp=" + resumeTimestamp +
           '}';
}
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Grades.java (1)

22-61: Implementation looks good, consider adding equals/hashCode methods.

The manual implementation of constructors, getters, setters, and toString is correct and follows good practices. The use of StringJoiner for toString is particularly good.

Consider adding equals/hashCode methods to ensure proper object equality comparison, especially important for collections and caching. Here's a suggested implementation:

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof Grades grades)) return false;
    return Objects.equals(grade, grades.grade) &&
           Objects.equals(date, grades.date) &&
           Objects.equals(score, grades.score);
}

@Override
public int hashCode() {
    return Objects.hash(grade, date, score);
}
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Restaurant.java (1)

50-128: Consider adding equals/hashCode methods for proper object comparison.

While the current implementation of constructors, getters, setters, and toString is correct, the class is missing equals/hashCode methods which are important for proper object comparison in collections.

Consider adding:

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof Restaurant that)) return false;
    return Objects.equals(id, that.id) &&
           Objects.equals(restaurantId, that.restaurantId) &&
           Objects.equals(name, that.name) &&
           Objects.equals(address, that.address) &&
           Objects.equals(borough, that.borough) &&
           Objects.equals(cuisine, that.cuisine) &&
           Objects.equals(grades, that.grades) &&
           Objects.equals(version, that.version);
}

@Override
public int hashCode() {
    return Objects.hash(id, restaurantId, name, address, borough, cuisine, grades, version);
}
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/controller/RestaurantController.java (1)

Line range hint 89-106: Consider making createRestaurant method fully reactive

The method currently mixes reactive and non-reactive code. Consider making it fully reactive for consistency with other endpoints.

-    @PostMapping
-    public ResponseEntity<GenericMessage> createRestaurant(
-            @RequestBody RestaurantRequest restaurantRequest) {
-        return ResponseEntity.created(
-                        URI.create(
-                                String.format(
-                                        "/restaurant/%s",
-                                        this.restaurantService
-                                                .createRestaurant(restaurantRequest)
-                                                .map(Restaurant::getName))))
-                .body(
-                        new GenericMessage(
-                                "restaurant with name %s created"
-                                        .formatted(restaurantRequest.name())));
-    }
+    @PostMapping
+    public Mono<ResponseEntity<GenericMessage>> createRestaurant(
+            @RequestBody RestaurantRequest restaurantRequest) {
+        return this.restaurantService
+                .createRestaurant(restaurantRequest)
+                .map(restaurant -> ResponseEntity
+                        .created(URI.create("/restaurant/" + restaurant.getName()))
+                        .body(new GenericMessage("restaurant with name %s created"
+                                .formatted(restaurantRequest.name()))));
+    }
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/controller/SearchController.java (1)

Line range hint 1-156: Consider adding OpenAPI documentation

The controller has multiple complex search endpoints with various parameters. Consider adding OpenAPI documentation using @operation and @parameter annotations to improve API discoverability and usage.

Example for one endpoint:

@Operation(summary = "Search restaurants within range", 
          description = "Find restaurants within specified distance from given coordinates")
@GetMapping("/search/restaurant/withInRange")
public Flux<ResultData> searchRestaurantsWithInRange(
        @Parameter(description = "Latitude coordinate") @RequestParam Double lat,
        @Parameter(description = "Longitude coordinate") @RequestParam Double lon,
        @Parameter(description = "Distance from coordinates") @RequestParam Double distance,
        @Parameter(description = "Unit of distance (km/mi)", example = "km") 
        @RequestParam(defaultValue = "km", required = false) String unit)
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/service/SearchService.java (1)

Line range hint 116-156: Consider extracting aggregation logic to separate class

The aggregation logic is complex and could benefit from being extracted into a dedicated class for better maintainability and testability.

Consider creating an AggregationProcessor class:

@Component
class AggregationProcessor {
    public Map<String, Map<String, Long>> processAggregations(
            Map<String, ElasticsearchAggregation> aggregationMap) {
        // Move existing aggregation logic here
    }
}
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/service/RestaurantService.java (2)

51-60: LGTM! Well-structured constructor replacing @requiredargsconstructor

The constructor implementation:

  • Uses constructor injection (Spring recommended approach)
  • Properly initializes all dependencies
  • Has a clear parameter order

Consider adding @NonNull annotations to the constructor parameters to ensure null-safety, especially since we're moving away from Lombok's built-in null checks.


Line range hint 1-300: Consider splitting responsibilities for better maintainability

While the implementation is functional, the service class has multiple responsibilities:

  • MongoDB operations
  • Elasticsearch operations
  • Change stream processing
  • Data transformation

Consider splitting this into separate services:

  • RestaurantMongoService: MongoDB operations
  • RestaurantSearchService: Elasticsearch operations
  • RestaurantStreamProcessor: Change stream processing
  • RestaurantMapper: Data transformation logic

This would:

  • Improve maintainability
  • Make the code more testable
  • Follow the Single Responsibility Principle

Would you like me to provide an example of how to refactor this into separate services?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 06741bf and 0ad02e5.

📒 Files selected for processing (14)
  • boot-mongodb-elasticsearch/pom.xml (5 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/ApplicationProperties.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/Initializer.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Address.java (2 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/ChangeStreamResume.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Grades.java (2 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Restaurant.java (2 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/elasticsearch/repository/CustomRestaurantESRepositoryImpl.java (3 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/mongodb/repository/CustomChangeStreamResumeRepositoryImpl.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/utils/DateUtility.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/controller/RestaurantController.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/controller/SearchController.java (1 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/service/RestaurantService.java (2 hunks)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/service/SearchService.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/Initializer.java
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/elasticsearch/repository/CustomRestaurantESRepositoryImpl.java
  • boot-mongodb-elasticsearch/pom.xml
  • boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Address.java
🔇 Additional comments (11)
boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/ChangeStreamResume.java (1)

14-28: LGTM! Clean implementation of getters and setters.

The explicit implementation of getters and setters is clean and follows Java beans specification correctly.

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/utils/DateUtility.java (1)

8-10: LGTM! Good removal of Lombok dependency

The change to an explicit static method is cleaner and removes the Lombok dependency, making the code more maintainable.

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/ApplicationProperties.java (1)

9-15: LGTM! Clean getter/setter implementation

The getter and setter methods for the cors property are well-implemented, following Java bean conventions. The default initialization of the cors field prevents potential null pointer exceptions.

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/document/Restaurant.java (1)

Line range hint 1-128: Verify Spring Boot 3.4.0 compatibility with dual indexing.

The class is configured for both MongoDB and Elasticsearch. While the implementation looks correct, we should verify compatibility with Spring Boot 3.4.0's versions of Spring Data MongoDB and Spring Data Elasticsearch.

✅ Verification successful

Dual indexing configuration is compatible with Spring Boot 3.4.0

The implementation is correctly configured for Spring Boot 3.4.0 with proper separation of MongoDB and Elasticsearch repositories:

  • Using Spring Boot 3.4.0 parent version with appropriate starters
  • Proper configuration of both MongoDB (replica set) and Elasticsearch connections
  • Clean separation of repositories using @EnableReactiveMongoRepositories and @EnableReactiveElasticsearchRepositories with correct filters
  • Reactive transaction support for MongoDB operations
  • Appropriate entity annotations for dual indexing
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any breaking changes in annotation usage
rg -l "@Document|@org.springframework.data.elasticsearch.annotations.Document" | xargs ast-grep --pattern 'class $_ {
  $$$
  @$_("$_")
  $$$
}'

# Check pom.xml for dependency versions
rg -A 5 "spring-boot-starter-data-mongodb|spring-boot-starter-data-elasticsearch" pom.xml

Length of output: 238


Script:

#!/bin/bash
# Check pom.xml for Spring Boot version and dependencies
rg -A 5 "<parent>|<artifactId>spring-boot" pom.xml

# Check for MongoDB and Elasticsearch dependencies
cat pom.xml

# Check for any other Restaurant classes or similar dual-indexed entities
fd -e java -x grep -l "org.springframework.data.elasticsearch.annotations.Document" {}
fd -e java -x grep -l "org.springframework.data.mongodb.core.mapping.Document" {}

Length of output: 4683


Script:

#!/bin/bash
# Check the module-specific pom.xml for Spring Boot version and dependencies
cat boot-mongodb-elasticsearch/pom.xml

# Check for any application.properties/yaml files that might have relevant MongoDB/Elasticsearch configuration
fd -e properties -e yml -e yaml -x cat {} boot-mongodb-elasticsearch/src/main/resources/

Length of output: 85425


Script:

#!/bin/bash
# Check application.properties/yaml for MongoDB and Elasticsearch configuration
find boot-mongodb-elasticsearch/src/main/resources -type f -name "application*.yml" -o -name "application*.yaml" -o -name "application*.properties" -exec cat {} \;

# Check for any test files that might show configuration
find boot-mongodb-elasticsearch/src/test -type f -name "*Test*.java" -exec grep -l "MongoDB\|Elasticsearch" {} \;

# Look for configuration classes
find boot-mongodb-elasticsearch/src/main/java -type f -name "*Config*.java" -exec cat {} \;

Length of output: 5262

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/controller/RestaurantController.java (1)

31-33: LGTM: Constructor-based dependency injection

The explicit constructor properly initializes the required dependency, following Spring best practices for dependency injection.

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/controller/SearchController.java (1)

23-25: LGTM: Constructor-based dependency injection

The explicit constructor properly initializes the required dependency, following Spring best practices for dependency injection.

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/service/SearchService.java (3)

30-32: LGTM: Constructor-based dependency injection

The explicit constructor properly initializes the required dependency, following Spring best practices for dependency injection.


21-21: LGTM: Using Spring's StringUtils

Good choice to use Spring's built-in StringUtils instead of Apache Commons, reducing external dependencies.


Line range hint 116-156: Add unit tests for aggregation logic

The complex aggregation logic warrants comprehensive unit testing to ensure correct handling of different aggregation types (sterms, dateRange).

Would you like me to help create unit tests for the aggregation logic?

✅ Verification successful

Unit tests for aggregation logic already exist and are comprehensive

The codebase already contains comprehensive unit tests for the aggregation logic in RestaurantESRepositoryIntegrationTest.java. Specifically:

  • The aggregation() test method thoroughly verifies the aggregation search functionality, including:
    • Correct handling of search results and pagination
    • Proper instantiation and type checking of ElasticsearchAggregations
    • Verification of aggregation map contents and structure
    • Validation of aggregation keys ("MyCuisine", "MyBorough", "MyDateRange")

The test coverage appears to be thorough and well-structured, testing both the search functionality and the aggregation logic together, which is appropriate given their tight coupling in the implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if tests exist for the aggregation logic
rg -l "class.*SearchServiceTest" 
rg "test.*aggregate.*search" 

Length of output: 65


Script:

#!/bin/bash
# Let's try a broader search for test files in the project
fd "Test.*\.java$"

# Also search for any test files containing "search" or "aggregate"
rg -l "class.*Test.*\{" | rg -l "search|aggregate" -i

Length of output: 20052


Script:

#!/bin/bash
# Let's search specifically in the boot-mongodb-elasticsearch module for test files related to search and aggregation
fd "Test.*\.java$" boot-mongodb-elasticsearch/src/test/

# Let's also look for test content specifically related to search aggregation in this module
rg -l "test.*[aA]ggregat" boot-mongodb-elasticsearch/src/test/
rg -C 3 "void test.*[sS]earch" boot-mongodb-elasticsearch/src/test/

Length of output: 895


Script:

#!/bin/bash
# Let's examine the content of the repository integration test file to see if there are any search/aggregation tests
rg -C 5 "search|aggregat" boot-mongodb-elasticsearch/src/test/java/com/example/mongoes/elasticsearch/repository/RestaurantESRepositoryIntegrationTest.java

# Also check the controller test for search/aggregation related tests
rg -C 5 "search|aggregat" boot-mongodb-elasticsearch/src/test/java/com/example/mongoes/web/controller/RestaurantControllerIntTest.java

Length of output: 19917

boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/service/RestaurantService.java (2)

25-26: LGTM! Clean migration from Lombok's @slf4j

The explicit logger declaration follows the standard SLF4J best practices.

Also applies to: 45-45


Line range hint 1-300: Verify reactive transaction behavior with Spring Boot 3.4.0

The upgrade to Spring Boot 3.4.0 might affect reactive transaction management. Let's verify the transaction behavior in the codebase.

✅ Verification successful

Let me gather more information about the transaction configuration and reactive MongoDB setup.


Let me check one more aspect regarding the transaction configuration and MongoDB client settings.


Spring Boot 3.4.0 reactive transaction configuration is compatible

The codebase has proper reactive transaction configuration with:

  • @EnableTransactionManagement and @EnableReactiveMongoRepositories annotations
  • ReactiveMongoTransactionManager bean configured correctly
  • Appropriate @Transactional usage with reactive types

The transaction management setup follows Spring Boot's recommended patterns and will continue to work correctly with Spring Boot 3.4.0.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential transaction-related changes in Spring Boot 3.4.0

# Search for other @Transactional usages with reactive types
rg -l "import org.springframework.transaction.annotation.Transactional" | xargs rg "@Transactional.*Mono|Flux"

# Check for any reactive MongoDB configuration
rg -l "ReactiveMongoTransactionManager|ReactiveMongoTemplate"

Length of output: 2471


Script:

#!/bin/bash
# Check for MongoDB configuration and transaction setup
ast-grep --pattern 'class DataStoreConfiguration {
  $$$
}'  boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/DataStoreConfiguration.java

# Check for transaction usage in RestaurantService
ast-grep --pattern '@Transactional
$$$
public $_ $_($$$) {
  $$$
}' boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/web/service/RestaurantService.java

# Check for any reactive transaction configuration
rg -A 5 "@EnableTransactionManagement|@EnableReactiveMongoRepositories"

Length of output: 1392


Script:

#!/bin/bash
# Check for MongoDB client and transaction configuration
rg -A 10 "ReactiveMongoClientFactoryBean|MongoTransactionManager|@Bean.*ReactiveMongoTemplate" boot-mongodb-elasticsearch/src/main/java/com/example/mongoes/config/DataStoreConfiguration.java

# Check for any transaction-related properties in application properties/yaml
fd -e yml -e yaml -e properties | xargs rg -i "spring\.data\.mongodb.*transaction|mongodb.*transaction"

Length of output: 1001

Comment on lines +8 to 10
public static LocalDateTime convertToLocalDateViaInstant(Date dateToConvert) {
return dateToConvert.toInstant().atZone(ZoneId.systemDefault()).toLocalDateTime();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add null check and consider timezone handling

The method could be more robust with null handling and explicit timezone management.

Consider this implementation:

     public static LocalDateTime convertToLocalDateViaInstant(Date dateToConvert) {
+        if (dateToConvert == null) {
+            return null;
+        }
         return dateToConvert.toInstant().atZone(ZoneId.systemDefault()).toLocalDateTime();
     }

Also consider adding Javadoc to document the null behavior:

    /**
     * Converts a {@link Date} to {@link LocalDateTime} using the system default timezone.
     *
     * @param dateToConvert the date to convert, can be null
     * @return the converted {@link LocalDateTime} or null if input is null
     */

Comment on lines +24 to +62
public String getPathPattern() {
return pathPattern;
}

public void setPathPattern(String pathPattern) {
this.pathPattern = pathPattern;
}

public String getAllowedMethods() {
return allowedMethods;
}

public void setAllowedMethods(String allowedMethods) {
this.allowedMethods = allowedMethods;
}

public String getAllowedHeaders() {
return allowedHeaders;
}

public void setAllowedHeaders(String allowedHeaders) {
this.allowedHeaders = allowedHeaders;
}

public String getAllowedOriginPatterns() {
return allowedOriginPatterns;
}

public void setAllowedOriginPatterns(String allowedOriginPatterns) {
this.allowedOriginPatterns = allowedOriginPatterns;
}

public boolean isAllowCredentials() {
return allowCredentials;
}

public void setAllowCredentials(boolean allowCredentials) {
this.allowCredentials = allowCredentials;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Security Risk: Permissive CORS Configuration Pattern

The verification reveals a concerning pattern across multiple modules in this repository. The same permissive CORS configuration is duplicated in over 20 different ApplicationProperties.java files, all using identical default values:

private String allowedMethods = "*";
private String allowedHeaders = "*";
private String allowedOriginPatterns = "*";
private boolean allowCredentials = true;

This configuration:

  1. Allows requests from any origin (*)
  2. Accepts all HTTP methods and headers
  3. Permits credentials (cookies, HTTP authentication)

No environment-specific configurations or security documentation was found to override these defaults. This combination creates a significant security risk as it completely bypasses the Same-Origin Policy protection.

Recommendations:

  • Remove the default values or use restrictive defaults
  • Add clear documentation about configuring CORS for production
  • Consider centralizing CORS configuration to avoid duplication
  • Add validation to prevent wildcard (*) usage when allowCredentials is true
🔗 Analysis chain

Review security implications of permissive CORS defaults

While the getter/setter implementations are correct, the default CORS configuration is highly permissive:

  • allowedMethods = "*"
  • allowedHeaders = "*"
  • allowedOriginPatterns = "*"
  • allowCredentials = true

This configuration might be suitable for development but could pose security risks in production.

Let's check if there are any production-specific CORS configurations:

Consider:

  1. Adding documentation about securing these values in production
  2. Using more restrictive defaults
  3. Implementing environment-specific CORS configurations
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for CORS configuration in application properties/yaml files
rg -l "cors" | grep -E "application.*\.(properties|ya?ml)$" | xargs cat

Length of output: 71


Script:

#!/bin/bash
# Let's try a broader search for CORS configuration files and properties
# First, find all application properties/yaml files
echo "=== Application Properties/YAML files ==="
fd -e properties -e yml -e yaml | grep -i "application"

# Then search for CORS-related code configurations
echo -e "\n=== CORS Configuration in Code ==="
rg -l --type java "cors" 

# Finally, let's look for any default values being set
echo -e "\n=== Default CORS Values ==="
rg -i "allowedMethods.*=.*\*|allowedHeaders.*=.*\*|allowedOriginPatterns.*=.*\*|allowCredentials.*=.*true" --type java

Length of output: 29410


Script:

#!/bin/bash
# Let's check if there are any environment-specific CORS configurations or documentation
echo "=== Checking for CORS configuration documentation ==="
rg -i "cors" README.md

# Look for WebSecurityConfigurerAdapter or SecurityFilterChain configurations
echo -e "\n=== Security configurations ==="
rg -l "WebSecurityConfigurerAdapter|SecurityFilterChain" --type java

# Check for any production profiles or configurations
echo -e "\n=== Production configurations ==="
fd -e properties -e yml -e yaml | grep -i "prod" | xargs cat 2>/dev/null

Length of output: 433

Comment on lines +100 to +106
public List<Grades> getGrades() {
return grades;
}

public void setGrades(List<Grades> grades) {
this.grades = grades;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve collection handling to prevent external modification.

The current implementation of getGrades() returns the internal list reference, which could lead to uncontrolled modifications of the internal state.

Consider implementing defensive copying:

public List<Grades> getGrades() {
-    return grades;
+    return new ArrayList<>(grades);
}

public void setGrades(List<Grades> grades) {
-    this.grades = grades;
+    this.grades = grades != null ? new ArrayList<>(grades) : new ArrayList<>();
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public List<Grades> getGrades() {
return grades;
}
public void setGrades(List<Grades> grades) {
this.grades = grades;
}
public List<Grades> getGrades() {
return new ArrayList<>(grades);
}
public void setGrades(List<Grades> grades) {
this.grades = grades != null ? new ArrayList<>(grades) : new ArrayList<>();
}

@rajadilipkolli rajadilipkolli merged commit 7ce45a3 into main Dec 5, 2024
6 of 7 checks passed
@rajadilipkolli rajadilipkolli deleted the mongoes branch December 5, 2024 12:40
this.changeStreamResumeRepository = changeStreamResumeRepository;
this.reactiveMongoTemplate = reactiveMongoTemplate;
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai, can you please rewrite private Flux<Restaurant> saveAll(List<String> restaurantStringList) method effectively and which is performant

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajadilipkolli, here's an optimized version of the saveAll method that enhances performance by leveraging reactive streams effectively:

private Flux<Restaurant> saveAll(List<String> restaurantStringList) {
    return Flux.fromIterable(restaurantStringList)
            .map(Document::parse)
            .map(this::documentToRestaurant)
            .flatMap(restaurantRepository::save);
}

private Restaurant documentToRestaurant(Document document) {
    Restaurant restaurant = new Restaurant();
    restaurant.setRestaurantId(Long.valueOf(document.get("restaurant_id", String.class)));
    restaurant.setName(document.get("name", String.class));
    restaurant.setCuisine(document.get("cuisine", String.class));
    restaurant.setBorough(document.get("borough", String.class));

    Address address = new Address();
    Document addressDoc = document.get("address", Document.class);
    address.setBuilding(addressDoc.get("building", String.class));
    address.setStreet(addressDoc.get("street", String.class));
    address.setZipcode(Integer.valueOf(addressDoc.get("zipcode", String.class)));
    List<Double> coord = addressDoc.getList("coord", Double.class);
    Point geoJsonPoint = new Point(coord.get(0), coord.get(1));
    address.setLocation(geoJsonPoint);
    restaurant.setAddress(address);

    List<Grades> gradesList = getGradesList(document.getList("grades", Document.class));
    restaurant.setGrades(gradesList);

    return restaurant;
}

Explanation:

  • Reactive Streams: By using Flux.fromIterable() and processing each item reactively, we avoid blocking operations and make better use of system resources.
  • Avoiding Intermediate Collections: Instead of collecting the data into a List and then saving, we process and save each Restaurant individually in a non-blocking manner with flatMap(restaurantRepository::save).
  • Code Organization: Extracting the mapping logic into a separate documentToRestaurant method improves readability and maintainability.

This approach should improve the performance, especially when dealing with large datasets, as it leverages the benefits of reactive programming to handle asynchronous data streams efficiently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: mongodb-elasticsearch type: dependency-upgrade Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant